Skip to content

Conversation

@linguini1
Copy link
Contributor

@linguini1 linguini1 commented Sep 12, 2025

Summary

Removes the default value for CONFIG_BOARD_LOOPSPERMSEC. All boards that forget to configure this value will encounter a build error, enforcing that configurations upstreamed to NuttX have calibrated values for correct delay timings.

Boards which implement ALARM_ARCH or TIMER_ARCH do not rely on the busy-loop delay implementation and thus only have a run-time check to ensure proper delay-timings (in the case where delays are used before the alarm/timer driver is registered).

Some boards already in the NuttX upstream do not have calibrated values, but there are no users who currently own the board to submit a calibrated value for the configurations to use. In this scenario, the value is temporarily set to 0 and a warning is displayed so that users of these boards are informed of the calibration process.

Closes #17004.

Impact

Users will no longer waste time debugging issues that result from having an incorrect value of this configuration option, causing delay/timing issues.

This will affect any user-created defconfig (or NuttX provided defconfig)
without a value set for CONFIG_BOARD_LOOPSPERMSEC by preventing them from
compiling. It causes files that use CONFIG_BOARD_LOOPSPERMSEC while it is
undefined to fail compilation.

Testing

On a board where CONFIG_BOARD_LOOPSPERMSEC=0 but the timer/alarm driver is not enabled, delays will not be reliable. This warning message is shown at compile time to tell the user how to calibrate the value:

$ ./tools/configure.sh nucleo-wl55jc:nsh
$ make -j
Create version.h
CC:  group/group_join.c clock/delay.c:63:2: warning: #warning "CONFIG_BOARD_LOOPSPERMSEC is set to 0 even though this architecture does" "not rely on timer or alarm drivers for correct timings. up_udelay() and " "similar delay functions will not work correctly. Please determine an " "appropriate value for CONFIG_BOARD_LOOPSPERMSEC using the calib_udelay " "application in nuttx-apps. If this configuration is a NuttX provided " "configuration, it would be appreciated if you submit a patch with the " "new value to apache/nuttx." [-Wcpp]
   63 | #warning                                                                     \
      |  ^~~~~~~
LD: nuttx
Memory region         Used Size  Region Size  %age Used
           flash:       63192 B       256 KB     24.11%
            sram:        5648 B        32 KB     17.24%
CP: nuttx.bin

On a board where ARCH_ALARM is used, no issues occur during compilation:

$ ./tools/configure.sh sim:nsh
$ make -j
LD:  nuttx
Pac SIM with dynamic libs..
'/usr/lib/libm.so.6' -> 'sim-pac/libs/libm.so.6'
'/usr/lib/libz.so.1' -> 'sim-pac/libs/libz.so.1'
'/usr/lib/libc.so.6' -> 'sim-pac/libs/libc.so.6'
'/usr/lib64/ld-linux-x86-64.so.2' -> 'sim-pac/libs/ld-linux-x86-64.so.2'
'/lib64/ld-linux-x86-64.so.2' -> 'sim-pac/ld-linux-x86-64.so.2'
SIM elf with dynamic libs archive in nuttx.tgz

On a board where CONFIG_BOARD_LOOPSPERMSEC is undefined, this is the compilation error:

$ ./tools/configure.sh nucleo-wl55jc:nsh
$ make -j
Create version.h
LN: platform/board to /home/linguini/coding/nuttx-space/apps/platform/dummy
Register: sh
Register: nsh
Register: dd
CC:  group/group_join.c clock/delay.c:59:6: warning: "CONFIG_BOARD_LOOPSPERMSEC" is not defined, evaluates to 0 [-Wundef]
   59 | #if (CONFIG_BOARD_LOOPSPERMSEC == 0) &&                                      \
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~
clock/delay.c:63:2: warning: #warning "CONFIG_BOARD_LOOPSPERMSEC is set to 0 even though this architecture does" "not rely on timer or alarm drivers for correct timings. up_udelay() and " "similar delay functions will not work correctly. Please determine an " "appropriate value for CONFIG_BOARD_LOOPSPERMSEC using the calib_udelay " "application in nuttx-apps. If this configuration is a NuttX provided " "configuration, it would be appreciated if you submit a patch with the " "new value to apache/nuttx." [-Wcpp]
   63 | #warning                                                                     \
      |  ^~~~~~~
CC:  group/group_setupidlefiles.c clock/delay.c: In function 'udelay_coarse':
clock/delay.c:92:23: error: 'CONFIG_BOARD_LOOPSPERMSEC' undeclared (first use in this function); did you mean 'CONFIG_BOARD_LOOPSPERUSEC'?
   92 |       for (i = 0; i < CONFIG_BOARD_LOOPSPERMSEC; i++)
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~
      |                       CONFIG_BOARD_LOOPSPERUSEC
clock/delay.c:92:23: note: each undeclared identifier is reported only once for each function it appears in
CC:  group/group_setuptaskfiles.c make[1]: *** [Makefile:65: delay.o] Error 1
make[1]: *** Waiting for unfinished jobs....
CC:  syslog/syslog_initialize.c make: *** [tools/LibTargets.mk:71: sched/libsched.a] Error 2
make: *** Waiting for unfinished jobs....
[linguini@pastabox nuttx]$ 

@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Sep 12, 2025
@linguini1
Copy link
Contributor Author

Looks like there are some defconfigs using the default value. Good to catch.

@cederom
Copy link
Contributor

cederom commented Sep 12, 2025

Looks like we need to fix the boards where -1 / default is used, just to pass build here and have correct timings on these boards, then update the res of the boards depending on what we have at hand? :-)

On the other hand we may keep this default and put some pressure / attention to the proper timings calibration when using different boards. I am wondering what impacts the value and if the differences are big - is it build specific? For instance when using different compiler, specific optimization level, many irqs, threads, etc :-)

I mean this 5000 default was set because any default may be wrong when this value is not calibrated on a final firmware build? People just did not know that? In that case we may leave as-is and just add Pre-Flight-Check-List to the documentation with list of important things to know / check / verify when creating / building NuttX based projects? :-)

@acassis
Copy link
Contributor

acassis commented Sep 12, 2025

@linguini1 instead of suggesting searching for CONFIG_BOARD_LOOPSPERMSEC I suggest search for calib_udelay because there is not a page dedicated to CONFIG_BOARD_LOOPSPERMSEC and when searching for it returns many pages.

Searching for calib_udelay returned two pages, but I think we need to move calib_udelay from apps/examples/ to apps/system/ because it is a system tool used to help the system to work correctly. And it also will avoid calib_udelay returning in the apps examples pages. What do you think?

@linguini1
Copy link
Contributor Author

@linguini1 instead of suggesting searching for CONFIG_BOARD_LOOPSPERMSEC I suggest search for calib_udelay because there is not a page dedicated to CONFIG_BOARD_LOOPSPERMSEC and when searching for it returns many pages.

That's fine, I plan to make one for BOARD_LOOPSPERMSEC but it's better to redirect to calib_udelay for now, I agree.

Searching for calib_udelay returned two pages, but I think we need to move calib_udelay from apps/examples/ to apps/system/ because it is a system tool used to help the system to work correctly. And it also will avoid calib_udelay returning in the apps examples pages. What do you think?

I don't mind moving it to the system directory but that's outside the scope of this PR. I agree it makes more sense, but I might do it later. Right now I just want to prevent users from making a frustrating mistake.

@linguini1
Copy link
Contributor Author

MSVC failure appears unrelated.

@acassis
Copy link
Contributor

acassis commented Sep 12, 2025

MSVC failure appears unrelated.

@simbit18 you are the MS guy, any idea?

acassis
acassis previously approved these changes Sep 12, 2025
@linguini1
Copy link
Contributor Author

@acassis @jerpelea do you think this change would be considered a "breaking change"?

@acassis
Copy link
Contributor

acassis commented Sep 12, 2025

@acassis @jerpelea do you think this change would be considered a "breaking change"?

No, it is not.

@github-actions github-actions bot added the Area: OS Components OS Components issues label Jan 16, 2026
@linguini1
Copy link
Contributor Author

@xiaoxiang781216 please let me know what you think of the current solution! If you think it's alright, I'll run the build checks once our usage has cooled down (as mentioned in #17914) to make sure everything is ready for merge.

Thank you for your patch in #17923. In this PR, you'll see I modified it slightly so that CONFIG_BOARD_LOOPSPERMSEC=0 by default when ALARM_ARCH/TIMER_ARCH are set in Kconfig instead of doing #ifdef -> #define in the C files. This should prevent build errors for other C files that use LOOPSPERMSEC too.

@linguini1
Copy link
Contributor Author

Thank you for the reviews @xiaoxiang781216, you have helped reduce the complexity of this PR. Please take a look at the new changes when you have a chance and let me know!

CONFIG_ARCH_CHIP_STM32WL55JC_CPU1=y
CONFIG_ARCH_CHIP_STM32WL5=y
CONFIG_BOARD_LATE_INITIALIZE=y
CONFIG_BOARD_LOOPSPERMSEC=0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it's better to make CONFIG_BOARD_LOOPSPERMSEC default to zero and warning or error in the source code, instead explicitly set it to zero in defconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because then it allows users to forget to add a proper value when they add a new config/board. The few places where I added zero to the defconfig is only a workaround for boards where no one owns them up come up with a real value. Otherwise this would break CI

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can warning/error in source code anyway. both are equal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I add an error in the source code it's more work; an undefined symbol automatically causes an error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you already add the check in code:
https://github.com/apache/nuttx/pull/17011/changes#diff-ebeffbbfc04cab0f5a0fd2ccbb776f2f541fcb836231537f55a27c503de761baR59
why do you duplicate the same thing in many places?

@linguini1
Copy link
Contributor Author

@simbit18 any idea why the CI build is failing for sim in the nuttx_kconfig.cmake file? I tried building a failing configuration (sim:nxffs) locally using CMake following the process from: https://nuttx.apache.org/docs/latest/quickstart/compiling_cmake.html and it worked without any warnings or errors.

@simbit18
Copy link
Contributor

simbit18 commented Jan 19, 2026

Hi @linguini1 first, please rebase the latest master

There are also errors that do not concern cmake.

====================================================================================
Configuration/Tool: qemu-armv8a/nsh_smp
2026-01-18 05:18:13
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Building NuttX...
smp_main.c: In function 'hog_milliseconds':
Error: smp_main.c:95:23: error: 'CONFIG_BOARD_LOOPSPERMSEC' undeclared (first use in this function)
   95 |       for (j = 0; j < CONFIG_BOARD_LOOPSPERMSEC; j++)
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~
smp_main.c:95:23: note: each undeclared identifier is reported only once for each function it appears in
make[2]: *** [/d/a/nuttx/nuttx/sources/apps/Application.mk:330: smp_main.c.d.a.nuttx.nuttx.sources.apps.testing.sched.smp.o] Error 1
make[2]: Target 'all' not remade because of errors.
make[1]: *** [Makefile:54: /d/a/nuttx/nuttx/sources/apps/testing/sched/smp_all] Error 2
make[1]: Target 'all' not remade because of errors.
make: *** [tools/LibTargets.mk:248: /d/a/nuttx/nuttx/sources/apps/libapps.a] Error 2
make: Target 'all' not remade because of errors.
/d/a/nuttx/nuttx/sources/nuttx/tools/testbuild.sh: line 385: /d/a/nuttx/nuttx/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  [1/1] Normalize qemu-armv8a/nsh_smp
make: *** [tools/Unix.mk:749: savedefconfig] Error 1
make: *** [tools/Unix.mk:726: olddefconfig] Error 1
====================================================================================

@linguini1
Copy link
Contributor Author

Hi @linguini1 first, please rebase the latest master

Okay, I'll try that.

There are also errors that do not concern cmake.

Yes, I'll have to fix those separately.

Removes the default value for CONFIG_BOARD_LOOPSPERMSEC. All boards that
forget to configure this value will encounter a build error, enforcing
that configurations upstreamed to NuttX have calibrated values for
correct delay timings.

Boards which implement ALARM_ARCH or TIMER_ARCH do not rely on the
busy-loop delay implementation and thus only have a run-time check to
ensure proper delay-timings (in the case where delays are used before
the alarm/timer driver is registered).

Some boards already in the NuttX upstream do not have calibrated values,
but there are no users who currently own the board to submit a
calibrated value for the configurations to use. In this scenario, the
value is temporarily set to 0 and a warning is displayed so that users
of these boards are informed of the calibration process.

Signed-off-by: Matteo Golin <[email protected]>
In Kconfig, it is valid for configuration variables to have no value.
However, the replace logic in this cmake file will throw an error that
at least four arguments are required if 'Value' is empty. This commit
avoids performing the replacement logic if 'Value' is empty to prevent
an error on valid, empty configuration variables.

Signed-off-by: Matteo Golin <[email protected]>
Propagated correct CONFIG_BOARDS_LOOPSPERMSEC value to remaining
configurations for this board.

Signed-off-by: Matteo Golin <[email protected]>
Use calibrated CONFIG_BOARD_LOOPSPERMSEC value for board configurations.

Signed-off-by: Matteo Golin <[email protected]>
Use calibrated CONFIG_BOARD_LOOPSPERMSEC value for board configurations.

Signed-off-by: Matteo Golin <[email protected]>
This board does not have a calibrated LOOPSPERMSEC value despite needing
one in order for correct operation. This commits sets the value to 0 so
that CI builds may pass, but users who build this board will see a
warning indicate the steps for calibrating a new value.

Signed-off-by: Matteo Golin <[email protected]>
This board does not have a calibrated LOOPSPERMSEC value despite needing
one in order for correct operation. This commits sets the value to 0 so
that CI builds may pass, but users who build this board will see a
warning indicate the steps for calibrating a new value.

Signed-off-by: Matteo Golin <[email protected]>
This board does not have a calibrated LOOPSPERMSEC value despite needing
one in order for correct operation. This commits sets the value to 0 so
that CI builds may pass, but users who build this board will see a
warning indicate the steps for calibrating a new value.

Signed-off-by: Matteo Golin <[email protected]>
This board does not have a calibrated LOOPSPERMSEC value despite needing
one in order for correct operation. This commits sets the value to 0 so
that CI builds may pass, but users who build this board will see a
warning indicate the steps for calibrating a new value.

Signed-off-by: Matteo Golin <[email protected]>
This board does not have a calibrated LOOPSPERMSEC value despite needing
one in order for correct operation. This commits sets the value to 0 so
that CI builds may pass, but users who build this board will see a
warning indicate the steps for calibrating a new value.

Signed-off-by: Matteo Golin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Build system Area: Drivers Drivers issues Area: OS Components OS Components issues Board: arm Board: arm64 Size: L The size of the change in this PR is large Size: M The size of the change in this PR is medium Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] CONFIG_BOARD_LOOPSPERMSEC should not have a default value